Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

First call of autodetect should be synchronous #1713

Merged
merged 2 commits into from
Jan 19, 2022
Merged

First call of autodetect should be synchronous #1713

merged 2 commits into from
Jan 19, 2022

Conversation

rubenvp8510
Copy link
Collaborator

Signed-off-by: Ruben Vargas [email protected]

Which problem is this PR solving?

  • Other initialization routines relies on platform value to do certain detection, in this particular case the oauth image was not detected correctly.

Short description of the changes

  • We need to make sure to detect the platform before doing any other initialization. In order to do that I made the first call of auto-detect synchronous (other calls will happens as usual, each 5 sec) . I think we should not have problems with this, but if this is a problem we can at least change the platform detection to be sync on the first call.

@codecov
Copy link

codecov bot commented Jan 19, 2022 •
edited
Loading

Codecov Report

Merging #1713 (e5cc447) into master (57d3d92) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1713      +/-   ##
==========================================
- Coverage   87.72%   87.71%   -0.02%     
==========================================
  Files          98       98              
  Lines        5963     5958       -5     
==========================================
- Hits         5231     5226       -5     
  Misses        556      556              
  Partials      176      176
Impacted Files Coverage Δ
pkg/autodetect/main.go 86.18% <100.00%> (-0.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57d3d92...e5cc447. Read the comment docs.

@rubenvp8510 rubenvp8510 force-pushed the autodetect-oauth-proxy branch from 3e7479b to bd20a92 Compare January 19, 2022 06:07
@rubenvp8510 rubenvp8510 force-pushed the autodetect-oauth-proxy branch from bd20a92 to def6407 Compare January 19, 2022 06:08
@rubenvp8510
Copy link
Collaborator Author

@pavolloffay Could you please review? it is a little bit hard to reach the coverage. Let me know if this is an issue I can try to fix it.

@rubenvp8510 rubenvp8510 enabled auto-merge (squash) January 19, 2022 15:18
@rubenvp8510 rubenvp8510 merged commit 343c4f2 into jaegertracing:master Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavolloffay pavolloffay pavolloffay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rubenvp8510 @pavolloffay